Add more order of operation comparisons #13
Conversation
Previously all BinOps were grouped under a single precedence enum "Op." Now, they are spread out and more fine grained. This should fix issues regarding parentheses and order of operations
src/subset/helpers.rs
Outdated
| Expr::IPath( | ||
| InstancePath::new(path), | ||
| Some(Rc::new(Expr::new_ref(index))), | ||
| ) |
There was a problem hiding this comment.
This looks like a formatting change. In general, we try to only commit the required functional changes for a particular commit. Can you remove this?
| use crate::util::pretty_print::{block, intersperse, PrettyHelper, PrettyPrint}; | ||
| use crate::util::pretty_print::{ | ||
| block, intersperse, PrettyHelper, PrettyPrint, | ||
| }; |
| .append(width.to_doc()) | ||
| .brackets(), | ||
| ) | ||
| } |
| RcDoc::text(".") | ||
| .append(RcDoc::as_string(id)) | ||
| .append(expr.to_doc().parens()) | ||
| }), | ||
| }, | ||
| ), |
src/util/pretty_print.rs
Outdated
| pub fn intersperse<'a>( | ||
| iter: impl Iterator<Item = RcDoc<'a>>, | ||
| separator: RcDoc<'a>, | ||
| ) -> RcDoc<'a> { |
src/v05/helpers.rs
Outdated
| Decl::Param( | ||
| name.to_string(), | ||
| Expr::new_ulit_dec(32, &value.to_string()), | ||
| ) | ||
| } |
There was a problem hiding this comment.
Both are formatting changes
There was a problem hiding this comment.
@nathanielnrn Seems like you might've committed a bunch of formatting changes. Can you separate them into a different PR or remove them entirely? Once done, I can re-review.
This reverts commit 9649010.
Sorry about that. |
src/subset/pretty_print.rs
Outdated
| (P::Index, P::Not) => Ordering::Less, | ||
| (P::Index, _) => Ordering::Greater, | ||
|
|
||
| (P::Mul, P::Not) | (P::Mul, P::Index) => Ordering::Less, |
There was a problem hiding this comment.
Since this code was written, rust started supporting nested patterns so you can write something like:
(P::Mul, P::Not | P::Index)
Which might be easier to read
|
yeah, seems like this project doesn't have a |
|
Made things nested, checks still fail due to other files' formatting from what I can tell, but should be good I think. |
|
That's really weird. The commands run for the workflow are here: https://github.com/vegaluisjose/vast/blob/main/Makefile Can you try running the rules in the Makefile and see if you can locally reproduce the errors? It might be the case that VSCode might be using a globally configured |
As discussed with @rachitnigam
Previously all BinOps were grouped under a single precedence enum "Op."
Now, they are spread out and more fine grained. This should fix issues
regarding parentheses and order of operations.
Also, some formatting was affected
Fixes #12